Description

GET /admin/update/status does not verify req.session.user.is_admin, and webaccess treats only
/admin-auth* as admin-protected, so with default requireAuthentication=false the endpoint can be
accessed unauthenticated and leaks currentVersion, latest, and vulnerableBelow. The included
test only forces auth by flipping requireAuthentication=true, which masks the default behavior.

Code

src/node/hooks/express/updateStatus.ts[R43-60]

+  // Admin-protected. webaccess.ts already gates /admin/* with admin auth.
+  app.get('/admin/update/status', async (_req: any, res: any) => {
+    const state = await loadState(stateFilePath());
+    const current = getEpVersion();
+    const installMethod = getDetectedInstallMethod();
+    const policy = state.latest
+      ? evaluatePolicy({installMethod, tier: settings.updates.tier, current, latest: state.latest.version})
+      : null;
+    res.json({
+      currentVersion: current,
+      latest: state.latest,
+      lastCheckAt: state.lastCheckAt,
+      installMethod,
+      tier: settings.updates.tier,
+      policy,
+      vulnerableBelow: state.vulnerableBelow,
+    });
+  });

Evidence

The route handler returns sensitive version and vulnerability data with no auth check. Global auth
middleware (webaccess.ts) only sets requireAdmin for paths starting with /admin-auth, and
otherwise allows access when settings.requireAuthentication is false (the default), so
/admin/update/status does not trigger an auth challenge. The integration test explicitly enables
requireAuthentication to make the request fail, indicating the route is not inherently protected.

src/node/hooks/express/updateStatus.ts[43-60]
src/node/hooks/express/webaccess.ts[58-135]
src/node/utils/Settings.ts[570-577]
src/tests/backend/specs/updateStatus.ts[76-87]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`/admin/update/status` is currently reachable without admin authentication under the default configuration (because webaccess only treats `/admin-auth*` as admin-protected and `requireAuthentication` defaults to false). This leaks operational/security-relevant information.
### Issue Context
Admin auth in this codebase is triggered by requesting `/admin-auth/*` (see `webaccess.ts`). New admin-only JSON endpoints should either:
- live under `/admin-auth/...` so the global middleware enforces admin auth, and/or
- explicitly check `req.session.user?.is_admin` and reject otherwise.
### Fix Focus Areas
- src/node/hooks/express/updateStatus.ts[43-60]
- admin/src/components/UpdateBanner.tsx[11-16]
- src/tests/backend/specs/updateStatus.ts[76-87]
### Expected change
1. Move the endpoint to `/admin-auth/update/status` **or** add an explicit admin check in the handler (prefer doing both for defense-in-depth).
2. Update the admin UI fetch URL accordingly.
3. Update/add an integration test that asserts the endpoint is unauthorized when `requireAuthentication` is left at its default (false) and no admin session is present.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

startPolling() schedules performCheck() via both setInterval and an initial setTimeout
without any in-flight guard, so slow GitHub fetches can cause concurrent performCheck() executions
that mutate the shared cached state and race on writing update-state.json. shutdown() clears
only the interval, not the initial timeout, so a check can still fire after shutdown has begun.

Code

src/node/updater/index.ts[R100-106]

+const startPolling = (): void => {
+  const intervalMs = Math.max(1, settings.updates.checkIntervalHours) * 60 * 60 * 1000;
+  if (timer) clearInterval(timer);
+  timer = setInterval(() => { void performCheck(); }, intervalMs);
+  // Run an immediate first check, but don't block boot.
+  setTimeout(() => { void performCheck(); }, 5000);
+};

Evidence

performCheck() mutates the singleton in-memory UpdateState returned by getCurrentState() and
persists it via saveState(). Because the poller triggers performCheck() without awaiting and
without locking, multiple invocations can interleave and write state out-of-order; additionally, the
initial timeout is not tracked so shutdown cannot cancel it.

src/node/updater/index.ts[20-25]
src/node/updater/index.ts[36-97]
src/node/updater/index.ts[100-121]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The updater poll loop can execute multiple `performCheck()` runs concurrently (interval tick + initial timeout, plus any later overlap if a check runs longer than the interval). Because `performCheck()` mutates shared in-memory state and writes it to disk, concurrent runs can cause state corruption/out-of-order writes and duplicate email decisions.
### Issue Context
`performCheck()` uses `getCurrentState()` which caches a singleton `inMemoryState`, then mutates it and calls `saveState()`.
### Fix Focus Areas
- src/node/updater/index.ts[36-106]
- src/node/updater/index.ts[118-121]
### Expected change
1. Add an in-flight guard (e.g., `let checkInFlight = false;`) so overlapping ticks return early or coalesce.
2. Store the `setTimeout()` handle and clear it in `shutdown()`.
3. Consider changing to a self-scheduling loop (`setTimeout` scheduled *after* an awaited `performCheck()` completes) to guarantee serialization.
4. (Optional) Add a unit/integration test that simulates a slow fetcher to ensure `performCheck()` is not entered concurrently.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The new /api/version-status and /admin/update/status routes are always registered, so behavior
changes even if updates.tier is set to off. This violates the requirement that disabling the
feature flag preserves prior behavior.

Code

src/node/hooks/express/updateStatus.ts[R29-60]

+export const expressCreateServer = (
+  _hookName: string,
+  {app}: ArgsExpressType,
+  cb: Function,
+): void => {
+  // Public endpoint. Cached for 60s. Returns only an enum — no version string.
+  app.get('/api/version-status', async (_req: any, res: any) => {
+    const now = Date.now();
+    if (now - badgeCache.at > BADGE_CACHE_MS) {
+      badgeCache = {value: await computeOutdated(), at: now};
+    }
+    res.json({outdated: badgeCache.value});
+  });
+
+  // Admin-protected. webaccess.ts already gates /admin/* with admin auth.
+  app.get('/admin/update/status', async (_req: any, res: any) => {
+    const state = await loadState(stateFilePath());
+    const current = getEpVersion();
+    const installMethod = getDetectedInstallMethod();
+    const policy = state.latest
+      ? evaluatePolicy({installMethod, tier: settings.updates.tier, current, latest: state.latest.version})
+      : null;
+    res.json({
+      currentVersion: current,
+      latest: state.latest,
+      lastCheckAt: state.lastCheckAt,
+      installMethod,
+      tier: settings.updates.tier,
+      policy,
+      vulnerableBelow: state.vulnerableBelow,
+    });
+  });

Evidence

PR Compliance ID 5 requires that when the feature flag is disabled, behavior matches what it was
before. The updateStatus hook unconditionally registers new routes, and ep.json unconditionally
enables the hook, so the new API/admin endpoints exist even when updates.tier is off.

src/node/hooks/express/updateStatus.ts[29-60]
src/ep.json[105-118]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`/api/version-status` and `/admin/update/status` are registered regardless of `settings.updates.tier`, so the updater feature cannot be fully disabled.
## Issue Context
Even if `updates.tier` is configured to `off`, the server still exposes new endpoints (a behavior change vs pre-PR). To meet the feature-flag requirement, the route-registration hook should no-op when `tier === 'off'`.
## Fix Focus Areas
- src/node/hooks/express/updateStatus.ts[29-60]
- src/ep.json[105-118]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

GET /admin/update/status is not guaranteed to be admin-only because webaccess only treats
/admin-auth* as “admin” and the default requireAuthentication=false allows the request through,
leaking currentVersion publicly. Even with authentication enabled, any authenticated non-admin can
hit the endpoint because requireAdmin is false for this path.

Code

src/node/hooks/express/updateStatus.ts[R43-59]

+  // Admin-protected. webaccess.ts already gates /admin/* with admin auth.
+  app.get('/admin/update/status', async (_req: any, res: any) => {
+    const state = await loadState(stateFilePath());
+    const current = getEpVersion();
+    const installMethod = getDetectedInstallMethod();
+    const policy = state.latest
+      ? evaluatePolicy({installMethod, tier: settings.updates.tier, current, latest: state.latest.version})
+      : null;
+    res.json({
+      currentVersion: current,
+      latest: state.latest,
+      lastCheckAt: state.lastCheckAt,
+      installMethod,
+      tier: settings.updates.tier,
+      policy,
+      vulnerableBelow: state.vulnerableBelow,
+    });

Evidence

The route returns currentVersion and other internal update metadata. The global auth middleware
only sets requireAdmin when the path starts with /admin-auth, and if authentication is not
required (the default) the middleware grants access to all paths; therefore /admin/update/status
is accessible without admin auth and can leak the running version.

src/node/hooks/express/updateStatus.ts[34-59]
src/node/hooks/express/webaccess.ts[58-60]
src/node/hooks/express/webaccess.ts[127-134]
src/node/utils/Settings.ts[570-577]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`/admin/update/status` currently relies on a comment assumption (“webaccess gates /admin/*”) that is not true with the current `webaccess.ts` logic. As a result, the endpoint can be reachable by unauthenticated users when `requireAuthentication=false` (default) and by non-admin authenticated users when `requireAuthentication=true`, leaking `currentVersion`.
### Issue Context
- `webaccess.ts` uses `requireAdmin = req.path.toLowerCase().startsWith('/admin-auth')`, so `/admin/update/status` is not treated as admin-only.
- The endpoint returns sensitive-ish info (`currentVersion`) that the PR explicitly tries not to expose publicly.
### Fix Focus Areas
- src/node/hooks/express/updateStatus.ts[29-60]
- src/node/hooks/express/webaccess.ts[58-60]
- src/node/hooks/express/webaccess.ts[127-134]
- src/tests/backend/specs/updateStatus.ts[76-87]
### Suggested fix
- In the `/admin/update/status` handler, require `req.session?.user?.is_admin === true`.
- If no session user: respond `401`.
- If session user exists but not admin: respond `403`.
- Add/extend integration tests to cover:
- default `requireAuthentication=false` still denies access
- authenticated non-admin user denied
- admin user allowed

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The new Express routes use async handlers directly, so exceptions (for example, loadState()
throwing on EPERM/IO errors) will not be forwarded to Express error handling in this codebase’s
established pattern. This can cause unhandled promise rejections and/or hung requests.

Code

src/node/hooks/express/updateStatus.ts[R35-41]

+  app.get('/api/version-status', async (_req: any, res: any) => {
+    const now = Date.now();
+    if (now - badgeCache.at > BADGE_CACHE_MS) {
+      badgeCache = {value: await computeOutdated(), at: now};
+    }
+    res.json({outdated: badgeCache.value});
+  });

Evidence

Other Express hooks in this repo wrap async logic in an IIFE and .catch(next) to safely propagate
errors; the new routes do not, and they call async functions that can throw (e.g., fs.readFile
failures in loadState).

src/node/hooks/express/updateStatus.ts[14-60]
src/node/hooks/express/importexport.ts[27-71]
src/node/updater/state.ts[22-29]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`updateStatus.ts` registers `async` Express handlers without wrapping them to forward rejections to `next(err)`. In this repo, other handlers explicitly `.catch(next)`, implying Express is not expected to handle async rejections automatically.
### Issue Context
`loadState()` can throw for non-ENOENT read failures (permissions, IO), and those errors would currently bypass Express error handling.
### Fix Focus Areas
- src/node/hooks/express/updateStatus.ts[29-60]
- src/node/hooks/express/importexport.ts[27-71]
### Suggested fix
- Change both routes to the established pattern:
- `app.get(path, (req, res, next) => { (async () => { ... })().catch((err) => next(err || new Error(err))); });`
- Optionally add a small helper (e.g. `wrapAsync`) local to the file to reduce duplication.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The pad footer badge fetches '/api/version-status' using an origin-root path, but the pad runtime
computes and uses pad.baseURL (derived from new URL('..', window.location.href).pathname) to
support being hosted under a subpath; this request will miss the prefix and 404 in such deployments.
As a result, the badge silently never appears even when the server reports severe/vulnerable.

Code

src/static/js/pad_version_badge.ts[R14-21]

+export const renderVersionBadge = async (): Promise<void> => {
+  const el = document.getElementById('version-badge');
+  if (!el) return;
+  try {
+    const res = await fetch('/api/version-status', {credentials: 'same-origin'});
+    if (!res.ok) return;
+    const data = (await res.json()) as BadgeResponse;
+    if (!data.outdated) { el.style.display = 'none'; return; }

Evidence

The pad bootstrap explicitly sets pad.baseURL to a computed basePath, and other pad code uses that
baseURL when calling server endpoints. The new badge code does not use the computed basePath, so it
won’t follow the same routing behavior under a subpath.

src/static/js/pad_version_badge.ts[14-22]
src/templates/padBootstrap.js[16-21]
src/static/js/pad.ts[907-913]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`pad_version_badge.ts` calls `/api/version-status` with an origin-root path, which breaks deployments where Etherpad is served under a subpath (because other pad requests are prefixed by `pad.baseURL`).
### Issue Context
`padBootstrap.js` derives `basePath` from the current URL and assigns it to `pad.baseURL`. Other pad code uses `${exports.baseURL}...` for server calls.
### Fix Focus Areas
- src/static/js/pad_version_badge.ts[14-22]
- src/templates/padBootstrap.js[16-21]
### Suggested fix
Build the request URL with the same basePath logic used by the pad bootstrap, e.g.:
- `const basePath = new URL('..', window.location.href).pathname;`
- `fetch(`${basePath}api/version-status`, ...)`
Avoid importing `./pad` from `pad_version_badge.ts` to prevent circular module initialization issues (it is already imported by `pad.ts`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

startPolling() schedules performCheck() via setInterval(() => { void performCheck(); }),
allowing overlapping checks when a run is slow. Concurrent runs can race on the shared in-memory
state object and collide on saveState()’s fixed filePath.tmp, risking state corruption and
email dedupe errors.

Code

src/node/updater/index.ts[R100-106]

+const startPolling = (): void => {
+  const intervalMs = Math.max(1, settings.updates.checkIntervalHours) * 60 * 60 * 1000;
+  if (timer) clearInterval(timer);
+  timer = setInterval(() => { void performCheck(); }, intervalMs);
+  // Run an immediate first check, but don't block boot.
+  setTimeout(() => { void performCheck(); }, 5000);
+};

Evidence

The polling loop does not guard against a previous check still running. The state writer always uses
the same tmp filename, so concurrent saves to the same target path are not safe.

src/node/updater/index.ts[36-106]
src/node/updater/state.ts[40-46]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The updater poller can run multiple `performCheck()` executions concurrently because the interval callback does not await completion and there is no mutex/in-progress flag. This can lead to concurrent mutation of the same `UpdateState` object and concurrent writes that share a fixed `${filePath}.tmp` path.
### Issue Context
Even though the default interval is 6h, slow/hung network requests or filesystem stalls can overlap with the next tick (and the initial `setTimeout` run can also overlap with the first interval tick).
### Fix Focus Areas
- src/node/updater/index.ts[36-106]
- src/node/updater/state.ts[40-46]
### Suggested fix
- Add a module-level `let inProgress = false;` guard in `performCheck()`:
- if `inProgress` return early
- set `inProgress = true` at start
- `finally { inProgress = false; }`
- Optionally:
- use a unique tmp filename (pid + timestamp) or a per-write random suffix to make `saveState()` robust even if concurrency happens again in the future.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

loadState() claims to return empty state for malformed files, but isValid() only checks
top-level types and can accept values that will later throw (e.g., vulnerableBelow: ["x"] or
latest: {}), crashing compareSemver()/isVulnerable() call sites. This makes
/api/version-status and notifier logic fragile to partially corrupted or hand-edited
var/update-state.json.

Code

src/node/updater/state.ts[R8-18]

+// NOTE: We validate top-level shape only. Subfields of `email` and `latest` are
+// trusted because EMPTY_STATE always provides them and only this module writes the file.
+// If a future consumer hand-edits the file, malformed subfields will surface at use site.
+const isValid = (raw: unknown): raw is UpdateState => {
+  if (!isPlainObject(raw)) return false;
+  return raw.schemaVersion === 1
+    && (raw.lastCheckAt === null || typeof raw.lastCheckAt === 'string')
+    && (raw.lastEtag === null || typeof raw.lastEtag === 'string')
+    && (raw.latest === null || isPlainObject(raw.latest))
+    && Array.isArray(raw.vulnerableBelow)
+    && isPlainObject(raw.email);

Evidence

isValid() allows any array for vulnerableBelow and any object for latest, but downstream code
assumes d.threshold and latest.version are strings. parseSemver() calls .trim() on its
input, which will throw if given undefined from a malformed state object that passed validation.

src/node/updater/state.ts[8-18]
src/node/updater/versionCompare.ts[13-27]
src/node/updater/versionCompare.ts[45-52]
src/node/hooks/express/updateStatus.ts[14-21]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`isValid()` is too permissive for fields that are later assumed to be well-typed, which undermines `loadState()`’s documented guarantee to safely reset malformed state.
### Issue Context
Downstream uses:
- `state.latest.version` passed into semver parsing
- `state.vulnerableBelow[].threshold` accessed for comparisons
### Fix Focus Areas
- src/node/updater/state.ts[5-19]
- src/node/updater/versionCompare.ts[13-52]
### Suggested fix
- Extend `isValid()` to validate:
- `latest` object shape when non-null (`version/tag/body/publishedAt/htmlUrl` strings; `prerelease` boolean)
- each `vulnerableBelow` entry is an object with `announcedBy` and `threshold` strings
- `email` object has `severeAt/vulnerableAt/vulnerableNewReleaseTag` as `string|null`
- If any subfield fails validation, treat the file as invalid and return `structuredClone(EMPTY_STATE)`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

New documentation content includes hard-coded https:// URLs instead of protocol-independent forms.
This violates the requirement to use protocol-independent URLs when introducing URLs in docs/code.

Code

docs/superpowers/plans/2026-04-25-auto-update-pr1-notify.md[R808-809]

+  html_url: 'https://github.com/ether/etherpad/releases/tag/v2.7.2',
+  published_at: '2026-04-25T00:00:00Z',

Evidence

PR Compliance ID 9 requires protocol-independent URLs (for example //...) when specifying URLs in
docs/code. The added planning document contains multiple https://... links.

docs/superpowers/plans/2026-04-25-auto-update-pr1-notify.md[808-809]
docs/superpowers/plans/2026-04-25-auto-update-pr1-notify.md[913-913]
docs/superpowers/plans/2026-04-25-auto-update-pr1-notify.md[2295-2295]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New documentation includes hard-coded `https://` URLs; the compliance checklist expects protocol-independent URL forms when URLs are introduced in docs/code.
## Issue Context
Update newly-added URLs in the affected doc(s) to use `//...` where appropriate.
## Fix Focus Areas
- docs/superpowers/plans/2026-04-25-auto-update-pr1-notify.md[808-809]
- docs/superpowers/plans/2026-04-25-auto-update-pr1-notify.md[913-913]
- docs/superpowers/plans/2026-04-25-auto-update-pr1-notify.md[2295-2295]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

performCheck() sets checkInFlight=true then awaits getCurrentState() outside the try/finally, so if
loadState() throws (e.g., EACCES) the promise rejects, checkInFlight is never reset, and future
polling ticks are permanently skipped (plus an unhandled rejection risk due to `void
performCheck()`).

Code

src/node/updater/index.ts[R38-45]

+const performCheck = async (): Promise<void> => {
+  if (settings.updates.tier === 'off') return;
+  // Coalesce overlapping ticks. performCheck mutates shared in-memory state and writes
+  // it to disk; concurrent runs would race on saveState() and could double-send emails.
+  if (checkInFlight) return;
+  checkInFlight = true;
+  const state = await getCurrentState();
+  try {

Evidence

checkInFlight is set before awaiting getCurrentState(), but the try/finally that resets it starts
after the await, so exceptions from getCurrentState()/loadState() bypass the reset; loadState()
explicitly rethrows non-ENOENT filesystem errors, and startPolling triggers performCheck without
awaiting/catching the returned promise.

src/node/updater/index.ts[38-46]
src/node/updater/index.ts[108-116]
src/node/updater/state.ts[21-29]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`performCheck()` can leave the updater permanently disabled if `getCurrentState()` throws before entering the `try/finally` that resets `checkInFlight`. This can also surface as an unhandled promise rejection because polling invokes `void performCheck()`.
## Issue Context
`loadState()` rethrows non-ENOENT fs errors (e.g., permission issues). If that happens during `getCurrentState()`, the `finally { checkInFlight = false }` never runs.
## Fix Focus Areas
- src/node/updater/index.ts[38-106]
- src/node/updater/index.ts[108-116]
## Implementation notes
- Move `const state = await getCurrentState()` inside the `try` block (or wrap the entire body in `try { ... } finally { ... }`).
- Optionally harden the timer callbacks to avoid unhandled rejections: `void performCheck().catch(err => logger.warn(...))` (even if performCheck should no longer reject).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

If the status endpoint is missing or returns non-OK (notably when updates.tier='off' disables route
registration), the admin UpdatePage never initiates a fetch and stays stuck on "Loading..."
indefinitely.

Code

admin/src/pages/UpdatePage.tsx[R4-10]

+export const UpdatePage = () => {
+  const {t} = useTranslation();
+  const us = useStore((s) => s.updateStatus);
+
+  if (!us) return <div>{t('admin.loading', {defaultValue: 'Loading...'})}</div>;
+
+  const upToDate = !us.latest || us.currentVersion === us.latest.version;

Evidence

The server hook explicitly skips registering /admin/update/status when tier is off. The UI always
shows the Update nav link, but UpdateBanner ignores non-OK responses and therefore never populates
the store; UpdatePage gates rendering on store presence and has no fallback/error state, so it can
remain permanently in the loading state.

src/node/hooks/express/updateStatus.ts[36-43]
admin/src/components/UpdateBanner.tsx[11-18]
admin/src/pages/UpdatePage.tsx[4-11]
admin/src/App.tsx[103-110]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The admin Update page can become unusable (permanent "Loading...") if `/admin/update/status` is not available or not OK (404 when `updates.tier === 'off'`, 401/403 when gated, network errors).
## Issue Context
- Server disables updater HTTP surface when tier is off.
- UI relies on UpdateBanner's background fetch to populate global store and does nothing on non-OK responses.
## Fix Focus Areas
- admin/src/pages/UpdatePage.tsx[4-12]
- admin/src/components/UpdateBanner.tsx[11-18]
- admin/src/App.tsx[103-118]
- src/node/hooks/express/updateStatus.ts[36-78]
## Implementation notes
Choose one coherent approach:
1) **UI-driven resilience (recommended):**
- Make `UpdatePage` fetch `/admin/update/status` itself (and set store) and render explicit states for 404 (updates disabled), 401/403 (not authorized), and generic errors.
- Update `UpdateBanner` to either (a) set an explicit error/disabled state on non-OK, or (b) keep it best-effort but ensure `/update` page is self-sufficient.
2) **Server-driven contract:**
- Keep the route registered even when tier is off, but return a minimal payload indicating `tier: 'off'` (and perhaps `policy: null`) so the UI can render a disabled message without hanging.
Also consider hiding the Update nav link once the UI knows updates are disabled (or not authorized).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The new updater subsystem is enabled by default because updates.tier defaults to notify, which
starts background polling on boot. This violates the requirement that new features be
feature-flagged and disabled by default to avoid changing behavior for existing deployments.

Code

src/node/utils/Settings.ts[R427-438]

+  /**
+   * Self-update subsystem (PR 1: tier 1 only).
+   * Tier "off" disables the version check entirely. Default "notify" shows a banner when behind.
+   */
+  upd...

Installation method detection with filesystem heuristics

• Detects how Etherpad was installed: docker (via /.dockerenv), git (.git + writable root), npm
 (writable package-lock.json), or managed (fallback).
• Respects explicit override setting; auto-detection runs only when override is 'auto'.
• Checks filesystem permissions to ensure detected method is actually writable.

src/node/updater/InstallMethodDetector.ts

Update capability policy evaluation engine

• Pure function evaluatePolicy() determines which update tiers are allowed based on install
 method, tier setting, and version comparison.
• Only 'git' is writable in PR 1; other methods are restricted to notify tier.
• Returns detailed PolicyResult with canNotify/canManual/canAuto/canAutonomous flags and
 human-readable reason.

src/node/updater/UpdatePolicy.ts

GitHub Releases API polling with ETag caching

• Fetches latest release from GitHub Releases API with ETag-based caching via If-None-Match.
• Parses release metadata and extracts vulnerable-below directives from release bodies.
• Skips prerelease versions but preserves ETag; handles rate-limiting (403/429) and network errors
 gracefully.
• Provides realFetcher using native Node 18+ fetch and a Fetcher interface for test stubbing.

src/node/updater/VersionChecker.ts

Email notification cadence and deduplication logic

• Implements email cadence logic: vulnerable repeats weekly, severe repeats monthly.
• decideEmails() is a pure function that returns planned emails and updated dedupe state without
 sending.
• Handles three email kinds: 'severe', 'vulnerable', 'vulnerable-new-release' (when a fix
 ships while vulnerable).
• Vulnerable status takes precedence over severe in the same tick.

src/node/updater/Notifier.ts

Updater orchestration and polling scheduler

• Main updater entry point wired via ep.json hooks expressCreateServer and shutdown.
• Manages polling interval (default 6h), in-memory state cache, and coalesces overlapping checks.
• Calls VersionChecker, evaluates UpdatePolicy, and invokes Notifier for email decisions.
• Placeholder sendEmailViaSmtp() logs (would send email) until real SMTP is wired in a follow-up
 PR.

src/node/updater/index.ts

HTTP endpoints for version status and admin updates

• Registers two HTTP endpoints: GET /api/version-status (public, cached 60s, returns only
 {outdated: null|'severe'|'vulnerable'}) and GET /admin/update/status (admin info with optional
 auth gate).
• Public endpoint deliberately does not leak the running version string to prevent fingerprinting.
• Admin endpoint returns full status including current version, latest release, install method,
 tier, and policy result.

src/node/hooks/express/updateStatus.ts

Settings schema for updater configuration

• Adds updates settings block with tier, source, channel, installMethod, checkIntervalHours,
 githubRepo, and requireAdminForStatus.
• Adds top-level adminEmail setting for admin notifications.
• Defaults: tier 'notify', installMethod 'auto', checkIntervalHours 6, githubRepo
 'ether/etherpad'.

src/node/utils/Settings.ts

Pad-side version status badge renderer

• Fetches /api/version-status on pad load and renders a footer badge only when outdated is
 'severe' or 'vulnerable'.
• Hardcoded English strings (TODO comment for future i18n via html10n).
• Auto-initializes on DOM ready; silently fails if fetch errors to never block pad load.

src/static/js/pad_version_badge.ts

HTML template for version badge container

• Adds <div id="version-badge"> element with ARIA attributes for status announcements.
• Initially hidden (display:none); shown by pad_version_badge.ts when needed.

src/templates/pad.html

CSS styling for pad version badge

• Styles #version-badge as a fixed bottom-right corner badge with conditional colors for severe
 (yellow) and vulnerable (red).
• Sets z-index 9999 to appear above pad content; max-width 320px for responsiveness.

src/static/css/pad.css

Admin UI banner component for update notifications

• React component that fetches /admin/update/status on mount and displays a banner when an update
 is available.
• Uses Zustand store to cache status; renders nothing if up-to-date or latest is null.
• Links to the dedicated /update page for full details.

admin/src/components/UpdateBanner.tsx

Admin UI page for update details and changelog

• Read-only admin page displaying current version, latest version, last check time, install method,
 and tier.
• Shows changelog (release body) and link to GitHub release when an update is available.
• Displays "up to date" message when running the latest version.

admin/src/pages/UpdatePage.tsx

Zustand store extension for update status

• Adds UpdateStatusPayload interface matching the shape of /admin/update/status response.
• Extends Zustand store with updateStatus state and setUpdateStatus setter.

admin/src/store/store.ts

Admin app navigation and banner integration

• Imports Bell icon from lucide-react and UpdateBanner component.
• Adds nav link to /update page with bell icon and i18n key 'update.page.title'.
• Renders UpdateBanner at the top of the main content area (below sidebar).

admin/src/App.tsx

Admin router configuration for update page

• Imports UpdatePage component and registers route path="/update" in the router.

admin/src/main.tsx

CSS styling for admin update UI components

• Adds .update-banner styles: yellow background, border, flex layout with gap and padding.
• Adds .update-page layout styles: definition list grid for key-value pairs, pre-formatted
 changelog display.

admin/src/index.css

English localization strings for update UI

• Adds 13 i18n keys under update.* namespace: banner title/body/CTA, page title/fields, changelog
 heading, up-to-date message, and badge text for severe/vulnerable.

src/locales/en.json

Pad initialization of version badge module

• Imports './pad_version_badge' to auto-initialize the badge renderer on pad load.

src/static/js/pad.ts

Settings template with updater configuration

• Adds updates block with default tier 'notify', source 'github', channel 'stable',
 installMethod 'auto', checkIntervalHours 6, githubRepo 'ether/etherpad', requireAdminForStatus
 false.
• Adds top-level adminEmail setting defaulting to null.

settings.json.template

Docker settings template with updater configuration

• Adds updates block identical to template except installMethod: 'docker' (explicit for
 clarity).
• Adds top-level adminEmail setting defaulting to null.

settings.json.docker

Plugin registration for updater and status endpoints

• Registers updater plugin with hooks expressCreateServer and shutdown pointing to
 src/node/updater/index.
• Registers updateStatus plugin (post-admin) with hook expressCreateServer pointing to
 src/node/hooks/express/updateStatus.

src/ep.json

Unit tests for version comparison logic

• Vitest unit tests for parseSemver(): valid versions, leading 'v', prerelease/build-metadata
 stripping, four-part rejection, garbage input.
• Tests for compareSemver(), isMajorBehind(), parseVulnerableBelow(), and isVulnerable()
 with multiple directives.

src/tests/backend-new/specs/updater/versionCompare.test.ts

Unit tests for state persistence

• Vitest tests for loadState() and saveState() with temp directories.
• Covers missing file, round-trip, corrupt JSON, unknown schema version, null email, array latest,
 atomic write, and directory creation.

src/tests/backend-new/specs/updater/state.test.ts

Unit tests for install method detection

• Vitest tests for detectInstallMethod() covering override honor, docker detection, git detection,
 npm detection, managed fallback, and precedence rules.

src/tests/backend-new/specs/updater/InstallMethodDetector.test.ts

Unit tests for update policy evaluation

• Vitest tests for evaluatePolicy() covering all tier/installMethod combinations,
 up-to-date/dev-build cases, and reason strings.

src/tests/backend-new/specs/updater/UpdatePolicy.test.ts

Unit tests for GitHub Releases API checker

• Vitest tests for checkLatestRelease() with mocked fetcher: 200 with parsing, 304 not-modified,
 403 rate-limit, prerelease skip, error handling, ETag passing, missing fields, null tag_name.

src/tests/backend-new/specs/updater/VersionChecker.test.ts

Unit tests for email notification cadence

• Vitest tests for decideEmails() cadence logic: first detection, re-emission after intervals,
 vulnerable precedence, tag-change detection, regression for tag-changed after cadence window.

src/tests/backend-new/specs/updater/Notifier.test.ts

Integration tests for update status endpoints

• Mocha integration tests for GET /api/version-status (no version leak, severe state) and `GET
 /admin/update/status` (shape, auth gate when requireAdminForStatus=true).
• Tests admin-only enforcement with hook-based auth mocking.

src/tests/backend/specs/updateStatus.ts

Playwright tests for pad version badge

• Playwright tests for pad footer badge: hidden on outdated:null, shows severe text with
 data-level attribute, shows vulnerable text with data-level attribute.

src/tests/frontend-new/specs/pad-version-badge.spec.ts

Playwright tests for admin update banner and page

• Playwright tests for admin update UI: nav link visibility, page renders with current version,
 banner appears when latest > current, changelog displays.

src/tests/frontend-new/admin-spec/update-banner.spec.ts

Admin sidebar navigation link count update

• Updates sidebar nav link count from 5 to 6 to account for new update page link.

src/tests/frontend-new/admin-spec/admintroubleshooting.spec.ts

Admin documentation for update subsystem

• New admin documentation covering tier 1 (notify) behavior, settings reference, outdated
 definitions, email cadence, pad badge visibility, privacy, and install-method detection heuristics.
• Notes that PR 1 ships cadence logic without real SMTP; tiers 2–4 are designed but not yet
 implemented.

doc/admin/updates.md

Complete design specification for auto-update system

• Comprehensive design specification for the four-tier auto-update system covering problem
 statement, goals, architecture, components, API surface, admin/pad UI, settings, data flow, error
 handling, state machine, security, testing strategy, and phased rollout plan.
• Establishes load-bearing decisions on update source (GitHub), execution model (in-process),
 rollback strategy, and tier capabilities.

docs/superpowers/specs/2026-04-25-auto-update-design.md

Changelog entry for tier 1 auto-update feature

• Adds "Unreleased" section documenting tier 1 (notify) launch: periodic GitHub polling, admin
 banner, pad badge, email cadence, new settings, and reference to doc/admin/updates.md.

CHANGELOG.md

Detailed task-by-task plan for auto-update Tier 1 implementation

• Comprehensive implementation plan for Tier 1 (notify) of the four-tier auto-update feature
• 18 sequential tasks covering backend modules, HTTP endpoints, settings, admin UI, pad UI, tests,
 and documentation
• Each task includes step-by-step instructions with code snippets, test patterns, and verification
 commands
• Covers pure modules (types.ts, versionCompare.ts, state.ts, InstallMethodDetector.ts,
 UpdatePolicy.ts, VersionChecker.ts, Notifier.ts), boot wiring, Express routes, admin React
 components, pad-side badge, i18n strings, and Playwright tests

docs/superpowers/plans/2026-04-25-auto-update-pr1-notify.md

Description

The new Express routes use async handlers directly, so exceptions (for example, loadState()
throwing on EPERM/IO errors) will not be forwarded to Express error handling in this codebase’s
established pattern. This can cause unhandled promise rejections and/or hung requests.

Code

src/node/hooks/express/updateStatus.ts[R35-41]

+  app.get('/api/version-status', async (_req: any, res: any) => {
+    const now = Date.now();
+    if (now - badgeCache.at > BADGE_CACHE_MS) {
+      badgeCache = {value: await computeOutdated(), at: now};
+    }
+    res.json({outdated: badgeCache.value});
+  });

Evidence

Other Express hooks in this repo wrap async logic in an IIFE and .catch(next) to safely propagate
errors; the new routes do not, and they call async functions that can throw (e.g., fs.readFile
failures in loadState).

src/node/hooks/express/updateStatus.ts[14-60]
src/node/hooks/express/importexport.ts[27-71]
src/node/updater/state.ts[22-29]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`updateStatus.ts` registers `async` Express handlers without wrapping them to forward rejections to `next(err)`. In this repo, other handlers explicitly `.catch(next)`, implying Express is not expected to handle async rejections automatically.

### Issue Context
`loadState()` can throw for non-ENOENT read failures (permissions, IO), and those errors would currently bypass Express error handling.

### Fix Focus Areas
- src/node/hooks/express/updateStatus.ts[29-60]
- src/node/hooks/express/importexport.ts[27-71]

### Suggested fix
- Change both routes to the established pattern:
 - `app.get(path, (req, res, next) => { (async () => { ... })().catch((err) => next(err || new Error(err))); });`
- Optionally add a small helper (e.g. `wrapAsync`) local to the file to reduce duplication.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The new updater subsystem is enabled by default because updates.tier defaults to notify, which
starts background polling on boot. This violates the requirement that new features be
feature-flagged and disabled by default to avoid changing behavior for existing deployments.

Code

src/node/utils/Settings.ts[R427-438]

+  /**
+   * Self-update subsystem (PR 1: tier 1 only).
+   * Tier "off" disables the version check entirely. Default "notify" shows a banner when behind.
+   */
+  updates: {
+    tier: 'notify',
+    source: 'github',
+    channel: 'stable',
+    installMethod: 'auto',
+    checkIntervalHours: 6,
+    githubRepo: 'ether/etherpad',
+  },

Evidence

PR Compliance ID 5 requires new features to be disabled by default. The default settings set
updates.tier to notify, the updater hook is registered in src/ep.json, and the updater starts
polling whenever the tier is not off, so the feature is active unless a deployer explicitly
disables it.

src/node/utils/Settings.ts[427-438]
settings.json.template[178-190]
src/ep.json[105-118]
src/node/updater/index.ts[108-116]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The updater subsystem is a new feature but is enabled by default because `updates.tier` defaults to `notify`, which starts polling on boot.
## Issue Context
Compliance requires new features to be behind a feature flag and disabled by default. The code currently supports disabling via `updates.tier: "off"`, but the default value is `notify` in both runtime defaults and the settings templates.
## Fix Focus Areas
- src/node/utils/Settings.ts[427-438]
- settings.json.template[178-190]
- settings.json.docker[187-200]
- src/node/updater/index.ts[108-116]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

GET /admin/update/status does not verify req.session.user.is_admin, and webaccess treats only
/admin-auth* as admin-protected, so with default requireAuthentication=false the endpoint can be
accessed unauthenticated and leaks currentVersion, latest, and vulnerableBelow. The included
test only forces auth by flipping requireAuthentication=true, which masks the default behavior.

Code

src/node/hooks/express/updateStatus.ts[R43-60]

+  // Admin-protected. webaccess.ts already gates /admin/* with admin auth.
+  app.get('/admin/update/status', async (_req: any, res: any) => {
+    const state = await loadState(stateFilePath());
+    const current = getEpVersion();
+    const installMethod = getDetectedInstallMethod();
+    const policy = state.latest
+      ? evaluatePolicy({installMethod, tier: settings.updates.tier, current, latest: state.latest.version})
+      : null;
+    res.json({
+      currentVersion: current,
+      latest: state.latest,
+      lastCheckAt: state.lastCheckAt,
+      installMethod,
+      tier: settings.updates.tier,
+      policy,
+      vulnerableBelow: state.vulnerableBelow,
+    });
+  });

Evidence

The route handler returns sensitive version and vulnerability data with no auth check. Global auth
middleware (webaccess.ts) only sets requireAdmin for paths starting with /admin-auth, and
otherwise allows access when settings.requireAuthentication is false (the default), so
/admin/update/status does not trigger an auth challenge. The integration test explicitly enables
requireAuthentication to make the request fail, indicating the route is not inherently protected.

src/node/hooks/express/updateStatus.ts[43-60]
src/node/hooks/express/webaccess.ts[58-135]
src/node/utils/Settings.ts[570-577]
src/tests/backend/specs/updateStatus.ts[76-87]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`/admin/update/status` is currently reachable without admin authentication under the default configuration (because webaccess only treats `/admin-auth*` as admin-protected and `requireAuthentication` defaults to false). This leaks operational/security-relevant information.
### Issue Context
Admin auth in this codebase is triggered by requesting `/admin-auth/*` (see `webaccess.ts`). New admin-only JSON endpoints should either:
- live under `/admin-auth/...` so the global middleware enforces admin auth, and/or
- explicitly check `req.session.user?.is_admin` and reject otherwise.
### Fix Focus Areas
- src/node/hooks/express/updateStatus.ts[43-60]
- admin/src/components/UpdateBanner.tsx[11-16]
- src/tests/backend/specs/updateStatus.ts[76-87]
### Expected change
1. Move the endpoint to `/admin-auth/update/status` **or** add an explicit admin check in the handler (prefer doing both for defense-in-depth).
2. Update the admin UI fetch URL accordingly.
3. Update/add an integration test that asserts the endpoint is unauthorized when `requireAuthentication` is left at its default (false) and no admin session is present.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

startPolling() schedules performCheck() via both setInterval and an initial setTimeout
without any in-flight guard, so slow GitHub fetches can cause concurrent performCheck() executions
that mutate the shared cached state and race on writing update-state.json. shutdown() clears
only the interval, not the initial timeout, so a check can still fire after shutdown has begun.

Code

src/node/updater/index.ts[R100-106]

+const startPolling = (): void => {
+  const intervalMs = Math.max(1, settings.updates.checkIntervalHours) * 60 * 60 * 1000;
+  if (timer) clearInterval(timer);
+  timer = setInterval(() => { void performCheck(); }, intervalMs);
+  // Run an immediate first check, but don't block boot.
+  setTimeout(() => { void performCheck(); }, 5000);
+};

Evidence

performCheck() mutates the singleton in-memory UpdateState returned by getCurrentState() and
persists it via saveState(). Because the poller triggers performCheck() without awaiting and
without locking, multiple invocations can interleave and write state out-of-order; additionally, the
initial timeout is not tracked so shutdown cannot cancel it.

src/node/updater/index.ts[20-25]
src/node/updater/index.ts[36-97]
src/node/updater/index.ts[100-121]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The updater poll loop can execute multiple `performCheck()` runs concurrently (interval tick + initial timeout, plus any later overlap if a check runs longer than the interval). Because `performCheck()` mutates shared in-memory state and writes it to disk, concurrent runs can cause state corruption/out-of-order writes and duplicate email decisions.
### Issue Context
`performCheck()` uses `getCurrentState()` which caches a singleton `inMemoryState`, then mutates it and calls `saveState()`.
### Fix Focus Areas
- src/node/updater/index.ts[36-106]
- src/node/updater/index.ts[118-121]
### Expected change
1. Add an in-flight guard (e.g., `let checkInFlight = false;`) so overlapping ticks return early or coalesce.
2. Store the `setTimeout()` handle and clear it in `shutdown()`.
3. Consider changing to a self-scheduling loop (`setTimeout` scheduled *after* an awaited `performCheck()` completes) to guarantee serialization.
4. (Optional) Add a unit/integration test that simulates a slow fetcher to ensure `performCheck()` is not entered concurrently.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The new /api/version-status and /admin/update/status routes are always registered, so behavior
changes even if updates.tier is set to off. This violates the requirement that disabling the
feature flag preserves prior behavior.

Code

src/node/hooks/express/updateStatus.ts[R29-60]

+export const expressCreateServer = (
+  _hookName: string,
+  {app}: ArgsExpressType,
+  cb: Function,
+): void => {
+  // Public endpoint. Cached for 60s. Returns only an enum — no version string.
+  app.get('/api/version-status', async (_req: any, res: any) => {
+    const now = Date.now();
+    if (now - badgeCache.at > BADGE_CACHE_MS) {
+      badgeCache = {value: await computeOutdated(), at: now};
+    }
+    res.json({outdated: badgeCache.value});
+  });
+
+  // Admin-protected. webaccess.ts already gates /admin/* with admin auth.
+  app.get('/admin/update/status', async (_req: any, res: any) => {
+    const state = await loadState(stateFilePath());
+    const current = getEpVersion();
+    const installMethod = getDetectedInstallMethod();
+    const policy = state.latest
+      ? evaluatePolicy({installMethod, tier: settings.updates.tier, current, latest: state.latest.version})
+      : null;
+    res.json({
+      currentVersion: current,
+      latest: state.latest,
+      lastCheckAt: state.lastCheckAt,
+      installMethod,
+      tier: settings.updates.tier,
+      policy,
+      vulnerableBelow: state.vulnerableBelow,
+    });
+  });

Evidence

PR Compliance ID 5 requires that when the feature flag is disabled, behavior matches what it was
before. The updateStatus hook unconditionally registers new routes, and ep.json unconditionally
enables the hook, so the new API/admin endpoints exist even when updates.tier is off.

src/node/hooks/express/updateStatus.ts[29-60]
src/ep.json[105-118]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`/api/version-status` and `/admin/update/status` are registered regardless of `settings.updates.tier`, so the updater feature cannot be fully disabled.
## Issue Context
Even if `updates.tier` is configured to `off`, the server still exposes new endpoints (a behavior change vs pre-PR). To meet the feature-flag requirement, the route-registration hook should no-op when `tier === 'off'`.
## Fix Focus Areas
- src/node/hooks/express/updateStatus.ts[29-60]
- src/ep.json[105-118]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

GET /admin/update/status is not guaranteed to be admin-only because webaccess only treats
/admin-auth* as “admin” and the default requireAuthentication=false allows the request through,
leaking currentVersion publicly. Even with authentication enabled, any authenticated non-admin can
hit the endpoint because requireAdmin is false for this path.

Code

src/node/hooks/express/updateStatus.ts[R43-59]

+  // Admin-protected. webaccess.ts already gates /admin/* with admin auth.
+  app.get('/admin/update/status', async (_req: any, res: any) => {
+    const state = await loadState(stateFilePath());
+    const current = getEpVersion();
+    const installMethod = getDetectedInstallMethod();
+    const policy = state.latest
+      ? evaluatePolicy({installMethod, tier: settings.updates.tier, current, latest: state.latest.version})
+      : null;
+    res.json({
+      currentVersion: current,
+      latest: state.latest,
+      lastCheckAt: state.lastCheckAt,
+      installMethod,
+      tier: settings.updates.tier,
+      policy,
+      vulnerableBelow: state.vulnerableBelow,
+    });

Evidence

The route returns currentVersion and other internal update metadata. The global auth middleware
only sets requireAdmin when the path starts with /admin-auth, and if authentication is not
required (the default) the middleware grants access to all paths; therefore /admin/update/status
is accessible without admin auth and can leak the running version.

src/node/hooks/express/updateStatus.ts[34-59]
src/node/hooks/express/webaccess.ts[58-60]
src/node/hooks/express/webaccess.ts[127-134]
src/node/utils/Settings.ts[570-577]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`/admin/update/status` currently relies on a comment assumption (“webaccess gates /admin/*”) that is not true with the current `webaccess.ts` logic. As a result, the endpoint can be reachable by unauthenticated users when `requireAuthentication=false` (default) and by non-admin authenticated users when `requireAuthentication=true`, leaking `currentVersion`.
### Issue Context
- `webaccess.ts` uses `requireAdmin = req.path.toLowerCase().startsWith('/admin-auth')`, so `/admin/update/status` is not treated as admin-only.
- The endpoint returns sensitive-ish info (`currentVersion`) that the PR explicitly tries not to expose publicly.
### Fix Focus Areas
- src/node/hooks/express/updateStatus.ts[29-60]
- src/node/hooks/express/webaccess.ts[58-60]
- src/node/hooks/express/webaccess.ts[127-134]
- src/tests/backend/specs/updateStatus.ts[76-87]
### Suggested fix
- In the `/admin/update/status` handler, require `req.session?.user?.is_admin === true`.
- If no session user: respond `401`.
- If session user exists but not admin: respond `403`.
- Add/extend integration tests to cover:
- default `requireAuthentication=false` still denies access
- authenticated non-admin user denied
- admin user allowed

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The new Express routes use async handlers directly, so exceptions (for example, loadState()
throwing on EPERM/IO errors) will not be forwarded to Express error handling in this codebase’s
established pattern. This can cause unhandled promise rejections and/or hung requests.

Code

src/node/hooks/express/updateStatus.ts[R35-41]

+  app.get('/api/version-status', async (_req: any, res: any) => {
+    const now = Date.now();
+    if (now - badgeCache.at > BADGE_CACHE_MS) {
+      badgeCache = {value: await computeOutdated(), at: now};
+    }
+    res.json({outdated: badgeCache.value});
+  });

Evidence

Other Express hooks in this repo wrap async logic in an IIFE and .catch(next) to safely propagate
errors; the new routes do not, and they call async functions that can throw (e.g., fs.readFile
failures in loadState).

src/node/hooks/express/updateStatus.ts[14-60]
src/node/hooks/express/importexport.ts[27-71]
src/node/updater/state.ts[22-29]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`updateStatus.ts` registers `async` Express handlers without wrapping them to forward rejections to `next(err)`. In this repo, other handlers explicitly `.catch(next)`, implying Express is not expected to handle async rejections automatically.
### Issue Context
`loadState()` can throw for non-ENOENT read failures (permissions, IO), and those errors would currently bypass Express error handling.
### Fix Focus Areas
- src/node/hooks/express/updateStatus.ts[29-60]
- src/node/hooks/express/importexport.ts[27-71]
### Suggested fix
- Change both routes to the established pattern:
- `app.get(path, (req, res, next) => { (async () => { ... })().catch((err) => next(err || new Error(err))); });`
- Optionally add a small helper (e.g. `wrapAsync`) local to the file to reduce duplication.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

loadState() claims to return empty state for malformed files, but isValid() only checks
top-level types and can accept values that will later throw (e.g., vulnerableBelow: ["x"] or
latest: {}), crashing compareSemver()/isVulnerable() call sites. This makes
/api/version-status and notifier logic fragile to partially corrupted or hand-edited
var/update-state.json.

Code

src/node/updater/state.ts[R8-18]

+// NOTE: We validate top-level shape only. Subfields of `email` and `latest` are
+// trusted because EMPTY_STATE always provides them and only this module writes the file.
+// If a future consumer hand-edits the file, malformed subfields will surface at use site.
+const isValid = (raw: unknown): raw is UpdateState => {
+  if (!isPlainObject(raw)) return false;
+  return raw.schemaVersion === 1
+    && (raw.lastCheckAt === null || typeof raw.lastCheckAt === 'string')
+    && (raw.lastEtag === null || typeof raw.lastEtag === 'string')
+    && (raw.latest === null || isPlainObject(raw.latest))
+    && Array.isArray(raw.vulnerableBelow)
+    && isPlainObject(raw.email);

Evidence

isValid() allows any array for vulnerableBelow and any object for latest, but downstream code
assumes d.threshold and latest.version are strings. parseSemver() calls .trim() on its
input, which will throw if given undefined from a malformed state object that passed validation.

src/node/updater/state.ts[8-18]
src/node/updater/versionCompare.ts[13-27]
src/node/updater/versionCompare.ts[45-52]
src/node/hooks/express/updateStatus.ts[14-21]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`isValid()` is too permissive for fields that are later assumed to be well-typed, which undermines `loadState()`’s documented guarantee to safely reset malformed state.
### Issue Context
Downstream uses:
- `state.latest.version` passed into semver parsing
- `state.vulnerableBelow[].threshold` accessed for comparisons
### Fix Focus Areas
- src/node/updater/state.ts[5-19]
- src/node/updater/versionCompare.ts[13-52]
### Suggested fix
- Extend `isValid()` to validate:
- `latest` object shape when non-null (`version/tag/body/publishedAt/htmlUrl` strings; `prerelease` boolean)
- each `vulnerableBelow` entry is an object with `announcedBy` and `threshold` strings
- `email` object has `severeAt/vulnerableAt/vulnerableNewReleaseTag` as `string|null`
- If any subfield fails validation, treat the file as invalid and return `structuredClone(EMPTY_STATE)`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools